-
-
Notifications
You must be signed in to change notification settings - Fork 363
revert(Dialog): revert InvokeAsync StateHasChanged #5632
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The head ref may contain hidden characters: "Dialog.Show\u5F02\u6B65\u6E32\u67D3_#5631"
Conversation
Reviewer's Guide by SourceryThis pull request fixes issue #5631 by modifying the Sequence diagram for Dialog.Show() asynchronous callsequenceDiagram
participant Dialog as Dialog
participant UI as UI
Dialog->>Dialog: Show(DialogOption option)
activate Dialog
Dialog->>Dialog: DialogParameters.Add(parameters, (_isKeyboard, _isBackdrop))
Dialog->>UI: InvokeAsync(StateHasChanged)
activate UI
UI-->>Dialog:
deactivate UI
deactivate Dialog
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Thanks for your PR, @densen2014. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @densen2014 - I've reviewed your changes - here's some feedback:
Overall Comments:
- The change from
Task Showtoasync Task Showis good, but it might be worth adding a comment explaining why this change is necessary.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5632 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 653 653
Lines 29344 29343 -1
Branches 4176 4179 +3
=========================================
- Hits 29344 29343 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Issues
fixes #5631
fixes #5633
Summary By Copilot
This pull request includes several changes to the
BootstrapBlazorproject, focusing on version updates and improvements to asynchronous handling in theDialogcomponent.Version Update:
src/BootstrapBlazor/BootstrapBlazor.csproj: Updated the project version from9.4.9-beta06to9.4.9-beta07.Asynchronous Handling Improvements:
src/BootstrapBlazor/Components/Dialog/Dialog.razor.cs: Modified theShowmethod to be asynchronous by changing its signature fromprivate Task Show(DialogOption option)toprivate async Task Show(DialogOption option).src/BootstrapBlazor/Components/Dialog/Dialog.razor.cs: ReplacedStateHasChanged()withawait InvokeAsync(StateHasChanged)to ensure state changes are invoked asynchronously.Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery